feat(trust): Phase 0 — shared TrustConfig + PlatformTrustConfigs by chaodu-agent · Pull Request #1266 · openabdev/openab

chaodu-agent · 2026-06-30T22:16:59Z

Phase 0 of the #1264 trust-pyramid implementation (plan).

Purely additive and behavior-preserving — defines the shared trust types + pure decision function. Not wired into AdapterRouter yet, so no runtime behavior changes.

Decision flow (what this module evaluates)

                 inbound (platform, channel_id, is_dm, sender_id)
                                   │
                                   ▼
                  PlatformTrustConfigs.get(platform)
                  (registered TrustConfig, else default:
                   L2 open / L3 deny-all)
                                   │
                                   ▼
          ┌─────────────────────────────────────────────┐
          │  TrustConfig::decide()                       │
          │                                             │
          │   L2 surface_allowed(channel_id, is_dm)?    │
          │     is_dm   → allow_dm                       │
          │     channel → allow_all_channels ||          │
          │               allowed_channels.contains      │
          │        │ no                                   │
          │        ▼                                     │
          │     Decision::DenyScope   (no echo)          │
          │        │ yes                                  │
          │        ▼                                     │
          │   L3 identity_allowed(sender_id)?           │
          │     allow_all_users ||                       │
          │     allowed_users.contains                   │
          │        │ no                                   │
          │        ▼                                     │
          │     Decision::DenyIdentity (echo UID) ◀──────┼── request-access UX
          │        │ yes                                  │
          │        ▼                                     │
          │     Decision::Allow → dispatch to ACP        │
          └─────────────────────────────────────────────┘

should_echo() is true only for DenyIdentity — scope denials are silent
(the surface simply isn't enabled), identity denials echo the sender their ID so
they can request access.

Phase 0 ships the box above as pure, tested logic. Phase 1 calls it from
AdapterRouter::handle_message() (with ChannelRef.is_dm + MessageContext.sender)
and removes the scattered per-adapter checks.

What's in this PR

crates/openab-core/src/trust.rs:
- TrustConfig — L2 scope (allow_all_channels/allowed_channels/allow_dm, default open) + L3 identity (allow_all_users/allowed_users, default deny-all)
- Decision enum — Allow / DenyScope / DenyIdentity; only DenyIdentity triggers the request-access echo (should_echo())
- PlatformTrustConfigs — registry keyed by platform() so a Telegram UID can never satisfy a LINE allowlist
- decide(channel_id, is_dm, sender_id) evaluates L2 then L3
11 unit tests covering the L2 × L3 × DM decision matrix

What's deferred

Phase 1: carrier changes (MessageContext.sender, ChannelRef.is_dm), wire decide() into handle_message(), remove scattered checks from discord.rs/slack.rs/gateway.rs
Phase 2: echo-on-deny + throttle
Phase 3: enforce the (already-default) deny-all at runtime — the breaking change is wiring deny-all into the live path, not changing the default value + migration

Testing

cargo clippy -p openab-core -D warnings clean
11/11 trust unit tests pass

Refs #1264

…itive) Defines the shared L2 (scope) + L3 (identity) trust types and pure decision function per the identity-trust-none ADR (#1264). Purely additive: not yet wired into AdapterRouter, changes no runtime behavior. - TrustConfig: L2 (allow_all_channels/allowed_channels/allow_dm, default open) + L3 (allow_all_users/allowed_users, default deny-all) - Decision enum (Allow / DenyScope / DenyIdentity) — only DenyIdentity echoes (request-access UX) - PlatformTrustConfigs registry keyed by platform() (no cross-platform ID bleed) - 11 unit tests covering the L2×L3×DM decision matrix Wiring + removing scattered per-adapter checks lands in Phase 1; the trust-none default flip lands in Phase 3. Refs #1264

chaodu-agent · 2026-06-30T22:29:29Z

LGTM ✅ — Purely additive Phase 0 introducing the shared trust decision model. Zero runtime behavior change, correct fail-closed logic, comprehensive test coverage.

What This PR Does

Introduces the L2 (scope) + L3 (identity) trust gate as a shared, pure module in openab-core. Not wired into any runtime path yet — this is foundational type + logic infrastructure for Phases 1–3 of the trust pyramid (#1264).

How It Works

TrustConfig encapsulates L2 channel/DM scope control and L3 user identity trust with ADR-correct defaults (L2 open, L3 deny-all).
Decision enum (Allow/DenyScope/DenyIdentity) encodes echo semantics — only identity denials trigger the request-access UX.
PlatformTrustConfigs registry keys configs by platform name, preventing cross-platform ID bleed.
decide() evaluates L2 → L3 in strict order: scope denial short-circuits before identity check (fail-closed, not fail-open).

Findings

#	Severity	Finding	Location
1	🟡	Phase 1 must apply trust gate to every dispatch path (including Thread/Lane batched modes via Dispatcher), not only `AdapterRouter::handle_message()`. Batched paths currently bypass the router entirely.	Phase 1 integration plan
2	🟡	`Decision` enum should add `#[non_exhaustive]` — Phase 2 will add rate-limit variants; without it, adding a variant is a semver-breaking change. Best to add now while there are zero downstream callers.	`trust.rs:28`
3	🟡	`DenyScope` doc comment says "the sender did not pass an authorization check" — misleading since L2 is explicitly NOT a security boundary per ADR. Suggest: "the bot is not configured to operate on this surface".	`trust.rs:33-36`
4	🟡	PR description "Phase 3: flip the L3 default to deny-all" contradicts the code which already defaults to deny-all. The breaking change is wiring deny-all into runtime enforcement, not changing the default. Description should be corrected.	PR description
5	🟡	`TrustConfig` fields are all `pub` — allows bypassing `new()` constructor and creating inconsistent states (e.g. `allow_all_channels = true` with non-empty `allowed_channels`). Consider `pub(crate)` or builder pattern before Phase 1.	`trust.rs:56-64`
6	🟡	`PlatformTrustConfigs` uses freeform `String` keys — case mismatch or typos in `platform()` return values would silently fall back to deny-all default. Consider `.to_lowercase()` normalization or a `Platform` enum.	`trust.rs:135-149`
7	🟡	Empty `sender_id` edge case — if Phase 1 adapters pass empty string (webhook/system messages), `identity_allowed("")` evaluates against the allowlist. Recommend an early-return `DenyIdentity` for empty sender_id, or a caller guard in Phase 1.	`trust.rs:103-105`
8	🟢	L2→L3 short-circuit is correctly fail-closed. L2 deny returns `DenyScope` immediately; the only `Allow` path requires both L2 and L3 to explicitly pass. No fail-open risk.	`trust.rs:108-115`
9	🟢	Default posture is secure: `allow_all_users: false` + empty `allowed_users` = deny-all. Unregistered platforms also fall back to deny-all.	`trust.rs:67-74`
10	🟢	Comprehensive 11-test decision matrix covering L2×L3×DM combinations, registry fallback, and echo semantics. Each test targets a specific edge case.	`trust.rs:152-279`
11	🟢	Platform-keyed registry prevents cross-platform ID bleed by design (Telegram UID cannot satisfy LINE allowlist).	`trust.rs:118-149`
12	🟢	Zero external dependencies — only `std::collections::{HashSet, HashMap}`. CI surface unchanged.	`trust.rs:21`
13	🟢	Module doc precisely scopes what's deferred (bot admission, trigger semantics) and what each phase delivers.	`trust.rs:1-19`
14	🟢	`decide()` rustdoc includes ASCII flow diagram matching the PR description's decision flow exactly.	`trust.rs:106-115`
15	🟢	Regression risk = zero. Module is `pub mod trust;` only — not called by any runtime code path.	`lib.rs`

Finding Details

🟡 F1: Phase 1 Integration Gap — Batched Dispatch Paths

Production adapters (Discord, Slack, Gateway) in Thread/Lane modes submit events directly to Dispatcher::submit → stream_prompt_blocks, bypassing AdapterRouter::handle_message(). Phase 1 must introduce a unified entry point (e.g. AdapterRouter::incoming_message) that evaluates the trust gate before routing to any dispatch path. This is not a Phase 0 bug (no runtime gate exists yet to bypass), but a critical design constraint for Phase 1.

🟡 F2: `#[non_exhaustive]` on Decision

Adding #[non_exhaustive] now (while there are zero downstream match statements) is free. Deferring it to Phase 2 would make the variant addition a breaking change for any code that exhaustively matches on Decision.

🟡 F3: DenyScope Doc Comment

Current: "the sender did not pass an authorization check"
Suggested: "the conversation surface is not enabled for this bot — this is scope control, not an authorization failure"

🟡 F4: PR Description Phase 3 Wording

The code already implements L3 deny-all as the default. Phase 3's breaking change is making this default enforced at runtime (wiring it in), not changing the default value itself.

🟡 F5–F7: Hardening for Phase 1

These are all non-blocking suggestions to address before Phase 1 wires the module into production paths: field visibility, key normalization, and empty-string guards.

What's Good (🟢)

Fail-closed by design: Every path either explicitly allows (both L2+L3 pass) or denies. No implicit allow.
Ergonomic API: Option<bool> flags + IntoIterator<Item = String> sets — flexible without builder boilerplate.
Test quality: 11 tests targeting specific edge cases in the decision matrix, not broad happy-path-only.
Documentation: Module doc clearly states phase boundaries, deferred items, and non-goals.
Isolation: Zero coupling to adapters, serde, or runtime config — clean Phase 0 boundary.
Echo semantics: should_echo() / is_allowed() helpers keep decision interpretation out of caller code.

Baseline Check

PR opened: 2026-06-30
Main already has: No trust.rs module — scattered per-adapter allowlist checks in individual adapter files
Net-new value: Unified, tested trust decision model that will replace scattered checks in Phase 1
Author: chaodu-agent (on behalf of maintainer)

Verdict rationale: All 🟡 findings are either Phase 1 pre-work suggestions (F1, F5–F7) or minor doc/API polish (F2–F4). The Phase 0 code itself is correct, secure (fail-closed), well-tested, and purely additive with zero runtime impact. Safe to merge as-is; findings should be addressed in Phase 1.

- F2: #[non_exhaustive] on Decision (avoid future semver break) - F3: reword DenyScope doc — scope control, not an authorization failure - F6: normalize platform keys to lowercase (case-insensitive registry) - F7: empty sender_id is never identity-allowed (fail-closed, even under allow_all_users) - F5: document new() as canonical constructor; allow_all_* takes precedence - add 3 tests (empty-sender, case-insensitive registry)

chaodu-agent · 2026-06-30T22:34:39Z

Thanks — addressed in ea65148:

#	Finding	Resolution
🟡 F2	`#[non_exhaustive]` on `Decision`	Done — added now while there are zero downstream matches.
🟡 F3	`DenyScope` doc wording	Done — now "scope control, not an authorization failure (L2 is not a security boundary), so it is silent."
🟡 F4	PR desc Phase 3 wording	Fixed — clarified the breaking change is wiring deny-all into the live path, not changing the default value.
🟡 F6	freeform `String` platform keys	Done — registry normalizes keys to lowercase on insert/get (case-insensitive); added a test.
🟡 F7	empty `sender_id` edge case	Done — `identity_allowed("")` now fails closed (returns false even under `allow_all_users`); added a test.
🟡 F5	field visibility / inconsistent states	Documented — `new()` is canonical; fields stay `pub` for cross-crate registry construction in Phase 1. Inconsistent combos are benign by precedence (an `allow_all_` flag always wins, so a stray `allowed_` list is simply ignored). No silent-deny risk.
🟡 F1	batched dispatch paths bypass router	Acknowledged for Phase 1 — agreed: the Thread/Lane `Dispatcher::submit` → `stream_prompt_blocks` paths bypass `handle_message()`. Phase 1 will introduce a single pre-dispatch entry point (e.g. `incoming_message`) that runs the gate before any dispatch path. Captured as a Phase 1 design constraint.

Tests: 13/13 trust tests pass, clippy -D warnings clean. F1/F5 are pre-Phase-1 design notes; everything actionable in Phase 0 is in.

chaodu-agent · 2026-06-30T22:35:32Z

LGTM ✅ — Clean, well-documented Phase 0 introducing the shared trust gate types with comprehensive tests.

What This PR Does

Introduces the L2 (scope) + L3 (identity) trust decision model as a purely additive, behavior-preserving module. It defines TrustConfig, Decision, and PlatformTrustConfigs — the shared foundation that Phase 1 will wire into AdapterRouter::handle_message().

How It Works

TrustConfig::decide(channel_id, is_dm, sender_id) evaluates L2 (surface scope) then L3 (identity trust) in sequence.
L2 defaults open (allow all channels + DMs); L3 defaults deny-all (no allowed users).
PlatformTrustConfigs is a case-insensitive registry keyed by platform name, preventing cross-platform ID bleed.
Decision::should_echo() returns true only for DenyIdentity — enabling the request-access UX in later phases.

Findings

#	Severity	Finding	Location
1	🟢	Correct fail-closed on empty `sender_id` even under `allow_all_users` — important security invariant	`trust.rs:120`
2	🟢	Case-insensitive platform lookup prevents subtle config mismatches	`trust.rs:149`
3	🟢	`#[non_exhaustive]` on `Decision` future-proofs the enum for throttle/rate-limit variants	`trust.rs:27`
4	🟢	Excellent test coverage — 11 tests covering the full L2 × L3 × DM decision matrix	`trust.rs:168-320`
5	🟢	Zero new dependencies — uses only `std` collections	`trust.rs:21`

Baseline Check

PR opened: 2026-06-30
Main already has: No trust-related code. lib.rs on main has no trust module.
Net-new value: Entire trust.rs module (320 lines) — shared types + pure decision function. Purely additive, no existing behavior changed.
Parent ADR: PR docs(adr): identity trust-none default & trust pyramid #1264 defines the trust pyramid architecture this implements.

What's Good (🟢)

ADR alignment: Implementation matches the trust pyramid ADR precisely — L2 scope (not security) → L3 identity (security gate).
Phase discipline: Clearly scoped to Phase 0 — types + tests only, no wiring. Module docs and comments explicitly state what's deferred.
Defensive defaults: L2 open + L3 deny-all is the correct secure-by-default posture.
Clean API surface: TrustConfig::new() with Option<bool> params makes the default-application explicit. Public fields for cross-crate construction while new() remains canonical.
Documentation: Module-level doc comments explain the layering rationale, what's in/out of scope, and link back to the ADR.

…ared gate (#1267) * feat(trust): Phase 1 (gateway) — route gateway ingress through shared trust gate Wires the shared L2/L3 gate (#1266) into the unified gateway path: - AdapterRouter gains a PlatformTrustConfigs registry (via with_trust builder — new()'s signature unchanged) + gate_incoming() ingress gate - process_gateway_event now enforces L2/L3 via router.gate_incoming(); should_skip_event keeps only bot-filter + @mention gating (its channel/user checks are neutered in the unified path) - registry built at startup from GATEWAY_* env, keyed per gateway platform Behavior-preserving: registry defaults mirror today's should_skip_event (allow-all default); is_dm passed false so DMs are evaluated as channels exactly as today. Discord/Slack routing + dispatch-path privatization + should_skip_event L2/L3 removal follow in later PRs. Deny-flip is Phase 3. Refs #1264 * docs(trust): add phase-2 TODO for is_dm carrier at gateway gate (review F2) --------- Co-authored-by: chaodu-agent <chaodu-agent@users.noreply.github.com>

chaodu-agent requested a review from thepagent as a code owner June 30, 2026 22:17

This comment has been minimized.

Sign in to view

github-actions Bot added the pending-maintainer label Jun 30, 2026

docs(trust): add compact decision-flow diagram to decide() rustdoc

3c507c3

This comment has been minimized.

Sign in to view

thepagent enabled auto-merge (squash) July 1, 2026 01:04

thepagent approved these changes Jul 1, 2026

View reviewed changes

thepagent merged commit da4d281 into main Jul 1, 2026
38 checks passed

This was referenced Jul 1, 2026

feat(trust): Phase 1 (gateway) — route gateway ingress through the shared gate #1267

Merged

trust: make the ingress gate structurally un-bypassable (Discord/Slack + privatize dispatch entries) #1269

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(trust): Phase 0 — shared TrustConfig + PlatformTrustConfigs#1266

feat(trust): Phase 0 — shared TrustConfig + PlatformTrustConfigs#1266
thepagent merged 3 commits into
mainfrom
feat/trust-phase0-shared-infra

chaodu-agent commented Jun 30, 2026 •

edited

Loading

Uh oh!

This comment has been minimized.

This comment has been minimized.

chaodu-agent commented Jun 30, 2026

🟡 F1: Phase 1 Integration Gap — Batched Dispatch Paths

🟡 F2: `#[non_exhaustive]` on Decision

🟡 F3: DenyScope Doc Comment

🟡 F4: PR Description Phase 3 Wording

🟡 F5–F7: Hardening for Phase 1

Uh oh!

chaodu-agent commented Jun 30, 2026

Uh oh!

chaodu-agent commented Jun 30, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

chaodu-agent commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Decision flow (what this module evaluates)

What's in this PR

What's deferred

Testing

Uh oh!

This comment has been minimized.

This comment has been minimized.

chaodu-agent commented Jun 30, 2026

What This PR Does

How It Works

Findings

🟡 F1: Phase 1 Integration Gap — Batched Dispatch Paths

🟡 F2: #[non_exhaustive] on Decision

🟡 F3: DenyScope Doc Comment

🟡 F4: PR Description Phase 3 Wording

🟡 F5–F7: Hardening for Phase 1

Uh oh!

chaodu-agent commented Jun 30, 2026

Uh oh!

chaodu-agent commented Jun 30, 2026

What This PR Does

How It Works

Findings

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

chaodu-agent commented Jun 30, 2026 •

edited

Loading

🟡 F2: `#[non_exhaustive]` on Decision